-
Notifications
You must be signed in to change notification settings - Fork 117
Clarify extended permission evaluation #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
I think the overall idea that this needs more clarification is good, but I do have some comments hoping to clarify and sharpen the explanation a bit before we merge.
src/xperm_rules.md
Outdated
considered. | ||
|
||
* If an extended permission rule is defined, the policy is first evaluated | ||
according to the high-level resource policy. For example: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above regarding "high-level resource policy". Additionally, I think this could spell out the situation more explicitly. After the AVC and constraint checks are performed, then the xperm checks will be applied.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thank you. I have updated this to clarify "standard AVC checks".
Adding documentation to clarify the automatic-deny evaluation when extended permissions are defined, as well as the overall evaluation logic. Signed-off-by: Liz Prucka <[email protected]>
Great, thank you for the review! I have attempted to clarify the explanation as per your comments, but please let me know if anything could use further clarification. |
Thanks for the updates! This seems good to me now. My typical practice is to leave approved PRs up for a week or two in case any other maintainers have comments and then merge via direct push. In this case, since this has been open for a while and other maintainers have had some time, I'll probably plan to merge later this week. |
Great, thank you! That sounds good to me. |
Adding documentation to clarify the automatic-deny evaluation when extended permissions are defined,
as well as the overall evaluation logic.